-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: execution methods into Executor class #3185
refactor: execution methods into Executor class #3185
Conversation
de88a51
to
495535f
Compare
I see your point, that is functionally equivalent, and it does seem cleaner. So....I think I have the wrapping execute and subscribe functions as slim as they can go, and I have combined the Executor and SubscriberExecutor classes into one. This is ready again for review. Please feel free to edit/rename/reformat within this PR as necessary. I am feeling very good about this! I think this will unlock a lot of potential for experimentation from which the ecosystem will benefit greatly. I'd love to see this within the next alpha. |
In particular, I don't love how I left the organization of folders/directories. The breakdown in structure between |
note that the Parser class is contained in lowercase parser.ts
in preparation for merging with Executor
makes execute file even thinner
in favor of union of ExecutionArgs & SubscriptionArgs
e85eed2
to
372d0eb
Compare
merging this into branch on main repo, will split out into separate PRs as possible |
see: #3163 (comment)
depends on #3184